[#190] Scene & SceneCoordinator 객체 리팩토링 by torch-ray · Pull Request #193 · SinsaStation/FullStackCodingBot

torch-ray · 2021-09-15T02:14:05Z

Scene 객체를 역할에 따라 다음과 같이 3분류로 나눴습니다.
- MainScene: Main, Shop, Rank, Item
- GameScene: Game, GameOver, Pause
- ControlScene: Alert, GameCenter, Setting, HowToPlay, Story
SceneType 인터페이스를 생성하였습니다.
- SceneCoordinator에 파라미터로 Main, Game, Control Scene을 모두 넣어 다형성을 구현하기 위함
- instantiate 메서드를 선언부 작성
- Main, Game, Control Scene이 SceneType 프로토콜 채택
SceneCoordinator transition 메서드의 파라미터 타입을 Scene -> SceneType으로 수정하였습니다.
SceneType 디렉토리를 생성하여 MainScene, GameScene, ControlScene, SceneType 파일을 이동하였습니다.

…정하고, 이에따라 필요한 코드 수정

torch-ray · 2021-09-15T02:15:11Z

자체적으로 디바이스 & 시뮬레이터에서 화면전환 테스트 완료했습니다.
물론 제가 테스트하긴 했지만 화면 전환 코드를 수정한 것이니만큼, 코드뿐만 아니라 화면전환이 정상동작되는지도 테스트 후에 머지 부탁드립니다.
천천히 리뷰해주세요 ~

torch-ray · 2021-09-15T02:16:18Z

아 머지 전에 추가로 개선 필요한 부분 있으시면, 말씀해주세요!
제 생각에 수정 필요한 부분은 다 개선했는데 더 있을 수도 있으니까요:)

eeeesong

시뮬레이터 돌려 봤고 이상 없어 보입니다!
고생 많으셨습니다 :)

eeeesong · 2021-09-15T03:41:37Z

+    case setting(MainViewModel)
+    case howToPlay(HowToPlayViewModel)
+    case story(StoryViewModel)
+}


Main과 Game이 나뉜 기준은 명확해보이는데, Control은 조금 불명확하게 다가옵니다.
setting, howToPlay, story는 sub-menu 같은 특성이 있는 것 같아서 같이 묶이는 게 맞는 것 같은데 gameCenter와 alert는 좀 애매하네요. 차라리 이 셋만 같이 두고, gameCenter와 alert만 따로 다루는 타입을 별도로 생성하는건 너무 사소하게 나누는 걸까요??
특히 alert는 확장성을 고려했을 때 따로 떨어져 있는 것이 좋을 것 같다는 생각도 듭니다!

그리고 Control이라는 작명자체도 조금 애매해보입니다!

사실 GameCenter도 alertf랑 분리되는게 맞을 거 같긴합니다. 쉽지 않네요... 한번 수정해보겠습니다! ㅎㅎ

eeeesong · 2021-09-15T03:42:57Z

+            guard var settingVC = storyboard.instantiateViewController(withIdentifier: IdentifierVC.setting) as? SettingViewController else {
+                Firebase.Analytics.logEvent("ViewControllerError", parameters: nil)
+                fatalError()
+            }


이제보니 ViewController를 생성하는 FactoryMethod를 따로 생성한다면 중복 코드를 줄일 수 있겠다는 생각이 드네요!
파베 로깅 구문도 한번만 쓸 수 있게 될 것 같고요

고려해서 수정방안 잡아보겠습니다 ㅎㅎ

이건 해보려고 했는데 쉽지 않네요. 각 뷰컨트롤러마다 타입에 맞는 타입 캐스팅이 필요하기 때문입니다. 그럼에도 만든다면 identifier를 파라미터로 받고 identifier에 따라 switch case로 분리하면 가능은 할 것 같은데, 오히려 코드가 훨씬 복잡해지는 느낌입니다. 좋은 방향이 있을까요?

음 제네릭하게 사용할 수 있는 메소드를 만들고 parameter로 타입을 받아도 해결되지 않을까요?
예전 프로젝트에서 작성했던 코드를 참고차 가져와보았습니다!

struct ViewControllerFactory { static func create<T: UIViewController>(from storyBoard: UIStoryboard, type: T.Type) -> T { let id = type.reuseIdentifier return storyBoard.instantiateViewController(withIdentifier: id) as? T ?? T() } } extension UIViewController { static var reuseIdentifier: String { return String(describing: self) } }

torch-ray · 2021-09-16T02:01:17Z

 import Firebase

-enum ControlScene {
+enum GameHelperScene {


@eeeesong

GameCenter만 따로빼고 GameHelperScene으로 묶었습니다. 사실 alert는 빼는게 좋은거 같은데, alert하나만 사용되는 객체를 따로 분리하는게 좋은지? 확신이 없어서 일단 보류했습니다.

그래도 alert를 빼는게 좋을거 같은지

GameHelperScene 객체명

두 부분에 대해 의견부탁드립니다.

1.확장성을 생각했을 때, 단순히 내용만 바꾸는 것 외의 alert가 추가될 수도 있으니 (예컨대 추후 추가할 수 있을 Game Update 유도) 빼놓는 게 나을 것 같습니다.
2. 객체명 적당하다고 생각합니다.

torch-ray · 2021-09-16T02:02:04Z

    case shop(ShopViewModel)
    case rank(RankViewModel)
    case item(ItemViewModel)
+    case gameCenter(UIViewController)


@eeeesong

gameCenter는 game과 main에서 고민했는데, 애초에 접근자체가 main에서 이루어지며 추후 아예 접근할 가능성이 없기 때문에 main이 적절하다고 판단해서 main으로 옮겼습니다. 의견부탁드립니다.

말씀하신 부분에 동의하며 main 쪽이 더 적절하다고 생각합니다!

eeeesong

말씀하신 부분들 확인했습니다.
제네릭 메소드 관련해서는 한번 시도해보면 좋을 것 같아요!

eeeesong · 2021-09-17T08:17:48Z

    case shop(ShopViewModel)
    case rank(RankViewModel)
    case item(ItemViewModel)
+    case gameCenter(UIViewController)


말씀하신 부분에 동의하며 main 쪽이 더 적절하다고 생각합니다!

eeeesong · 2021-09-17T08:21:43Z

 import Firebase

-enum ControlScene {
+enum GameHelperScene {


1.확장성을 생각했을 때, 단순히 내용만 바꾸는 것 외의 alert가 추가될 수도 있으니 (예컨대 추후 추가할 수 있을 Game Update 유도) 빼놓는 게 나을 것 같습니다.
2. 객체명 적당하다고 생각합니다.

eeeesong · 2021-09-17T08:22:50Z

+protocol SceneType {
+
+    func instantiate(from identifier: String) -> UIViewController
+}


이렇게 protocol에 메소드 하나만 정의 된 경우에
메소드 위 줄간격 붙이는 건 어떨까요?ㅎㅎ

torch-ray added 7 commits September 15, 2021 10:45

refactor: [#190] Scene을 Main, Game, Control로 객체 분리

63171d1

chore: [#190] 메서드명 오타 수정

5e2dc79

feat: [#190] SceneType 인터페이스 생성, 추후 coordinator의 파라미터로 사용하기 위함

7047a55

refactor: [#190] Setting, HowToPlay, Story Scene Control Scene으로 이동

aace525

refactor: [#190] SceneCoordinatorType default implemantation 메서드 코드 수…

05660f9

…정하고, 이에따라 필요한 코드 수정

chore: [#190] 불필요한 수정전 객체인 Scene 파일 삭제

3cbab0c

chore: [#190] Scene 전환 정상동작 테스트 및 디렉토리 변경

fc56232

torch-ray added refactoring refactoring seok labels Sep 15, 2021

torch-ray added this to the 1.05 milestone Sep 15, 2021

torch-ray self-assigned this Sep 15, 2021

eeeesong reviewed Sep 15, 2021

View reviewed changes

refactor: [#190] GameHelperScene으로 이름변경 후, GameCenter는 MainScene으로 이동

15e6703

torch-ray commented Sep 16, 2021

View reviewed changes

eeeesong reviewed Sep 17, 2021

View reviewed changes

torch-ray and others added 2 commits September 24, 2021 12:15

refactor: [#190] AlertScene 분리 및 프로토콜 띄어쓰기 변경

fa29e5b

Merge branch 'iOS/dev' into refactor/scene

450b7a1

eeeesong merged commit e9e0e7f into iOS/dev Sep 24, 2021

Uh oh!

Conversation

torch-ray commented Sep 15, 2021

Uh oh!

torch-ray commented Sep 15, 2021

Uh oh!

torch-ray commented Sep 15, 2021

Uh oh!

eeeesong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eeeesong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants